Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct docs generation #252

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Conversation

holly-cummins
Copy link
Contributor

See discussion in #236 (comment)

@gsmet, thank you for your help. Does this look better?
I think I now understand the intention of the changes :)

(... although I still don't understand how my previous version built cleanly.)

@holly-cummins holly-cummins requested a review from a team as a code owner November 6, 2024 15:13
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

quarkus-config-doc-maven-plugin is only used in the docs module. That's where you need the version defined. And that's what it does with this commit given the the version in the pluginManagement of the root pom will be inherited by the docs module.

With the previous version, you were defining the version in modules where you didn't use the plugin.

Now, things can work for several reasons:

  • if you enforce the version in the docs module directly when declaring the plugin
  • if you're lucky enough that the latest version of the quarkus-config-doc-maven-plugin (that's what Maven will use if no version is defined) is compatible with what the version of the extension annotation processor will produce. So it might work for a few versions and then suddenly get broken. It could also be broken in a subtle way that doesn't fail the build.

The version of quarkus-config-doc-maven-plugin should be the same as the version of the extension annotation processor.

@holly-cummins
Copy link
Contributor Author

Ahh, thanks @gsmet. It was probably picking up the latest, and that's why it seemed to work.

@holly-cummins holly-cummins merged commit 4d562ae into quarkiverse:main Nov 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants